Allow borrowing array elements from packed structs with ABI align <= packed align #145419

hns1971 · 2025-08-15T03:12:11Z

Summary

This PR fixes a false positive E0793 ("reference to packed field is unaligned")
when borrowing from a #[repr(C, packed)] struct field whose type is a const-generic array
with an element type that has ABI alignment <= the struct's packed alignment.

Example before this change:

#[repr(C, packed)]
struct PascalString<const CAP: usize> {
    len: u8,
    buf: [u8; CAP],
}

fn bar<const CAP: usize>(s: &PascalString<CAP>) -> &str {
    std::str::from_utf8(&s.buf[0..s.len as usize]).unwrap()
}

- Add fallback mechanism in is_disaligned() when layout computation fails - Handle TooGeneric errors by computing element alignment from type structure - Fix packed-array-const-generic.rs test to pass compilation - Resolves issue where const generic parameters couldn't be resolved during alignment checks

rustbot · 2025-08-15T03:12:16Z

r? @SparrowLii

rustbot has assigned @SparrowLii.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-08-15T03:12:18Z

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

gralpli

Hey, I’m the bug reporter. Just what I noticed on a first glance.

compiler/rustc_const_eval/src/util/alignment.rs

tests/ui/packed/packed-array-const-generic.rs

lcnr · 2025-08-15T14:06:37Z

compiler/rustc_const_eval/src/util/alignment.rs

+        Ok(normalized) => normalized,
+        Err(_) => {
+            // If normalization fails, fall back to the original type
+            ty


why? it feels a lot safer to conservatively return true here, same as when layout_of fails

still relevant

lcnr · 2025-08-15T14:50:03Z

compiler/rustc_const_eval/src/util/alignment.rs

+            // For const generic arrays like [u8; CAP], we can make a reasonable assumption
+            // about their alignment based on the element type.


we're not making a reasonable assumption. Their layout literally only depend son the layout of their element type, doesn't it?

Getting this right is soundness critical, so we need to be confident about that we don#t introduce false negatives.

Also, please rename this function to is_potentially_disaligned, to clarify that it's allowed to return false positives, but not false nnegatives

We use "misaligned" elsewhere in the compiler, maybe let's go with that instead of "disaligned" (which I never saw/heard before).

also still relevant

Thanks for the detailed feedback!

I agree with the soundness concern and with using the term “misaligned”.
I will:

rename the function to is_potentially_misaligned and update call sites/docs to clarify that it may return false positives but must not return false negatives (soundness-critical),

avoid relaxing the check when layout_of(ty) fails in general.

To still address the motivating case without introducing false negatives, I’ll only add a very small, layout-free special-case:
when ty.kind() is Array(elem, _) and elem is u8 or i8 (which have ABI alignment 1 by definition, independent of the array length), we treat the borrow as not potentially misaligned. All other cases remain conservative and return true when layout_of(ty) fails.

This fixes the [u8; CAP] packed-struct case while keeping the function strictly conservative for any type whose ABI alignment could exceed the packed alignment.

If you’d prefer the even stricter variant (no special-casing at all), I can drop the exception and keep returning true on layout_of failure across the board.

I think ignoring the length and only checking the argument is fine. My issue was with the comment, not the behavior. We already guarantee that the alignment constraints are equal:

you can get a &T out of an &[T], so [T] must have stronger requirements than &T

you can get a &[T] from a &T via std::array::from_ref , so T must have stronger requirements than &[T]

Thanks! I’ve updated the comment to formally justify that align([T]) == align(T),
using the two directions you mentioned (&[T] -> &T and std::array::from_ref(&T)).
Behavior remains unchanged: we only consider the element type’s ABI alignment and
ignore the array length.

… clarity; Unified to ty::Array(element_ty, _) | ty::Slice(element_ty) => …; Modify Chinese comments to English comments

… clarity

rustbot · 2025-08-18T05:21:52Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

hns1971 · 2025-08-18T05:24:06Z

Fixed:

Rename is_disaligned to is_potentially_disaligned for better semantic clarity
Unified to ty::Array(element_ty, _) | ty::Slice(element_ty) => …
Modify Chinese comments to English comments.

Thanks

hns1971 · 2025-08-20T01:48:36Z

I've updated the PR as requested (renamed function, updated comment, removed special-casing). Just let me know if further adjustments are needed.

lcnr · 2025-08-22T08:59:18Z

compiler/rustc_const_eval/src/util/alignment.rs

+            // Only allow u8 and i8 arrays when layout computation fails
+            // Other types are conservatively assumed to be misaligned
+            match element_ty.kind() {


properly compute the of the element_ty here, limiting it to only u8 and i8 feels unnecessary 🤔

lcnr · 2025-08-22T09:00:42Z

tests/ui/packed/packed-array-i8-const-generic.rs

+#![allow(dead_code)]
+
+#[repr(C, packed)]
+struct PascalStringI8<const CAP: usize> {


please merge this into a single test and use generic-length instead of const-generic in the file name

lcnr · 2025-08-22T09:04:30Z

cc @rust-lang/lang this causes us to no longer lint "due to a borrow of a potentially misaligned field" when borrowing from a [sufficiently_small_alignment; N] field. We previous failed to compute the layout of that array but now manually check the alignment of the array element type.

I feel like this may be fine without an FCP as it seems like a small enough change 🤷

traviscross · 2025-08-22T10:40:07Z

Thanks for the ping. Specifically, we were giving a hard error here, so allowing this is a stabilization.

Since we're relatively caught up, it'll be as fast for us to FCP this as to decide not to do so, and lately we've generally been leaning toward FCPing everything that we technically should. We could waive our 10-day period, but it doesn't seem likely that'll affect what release this lands in, so let's just, in the normal way...

@rfcbot fcp merge
@rustbot labels +I-lang-easy-decision

rfcbot · 2025-08-22T10:40:10Z

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

exact-rules-list (Allow borrowing array elements from packed structs with ABI align <= packed align #145419 (comment))
semver hazard (Allow borrowing array elements from packed structs with ABI align <= packed align #145419 (comment))

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

lcnr · 2025-08-22T11:12:43Z

Oh, yeah, thought this was a lint 👍 an FCP seems appropriate

scottmcm · 2025-08-22T18:10:57Z

Yeah, if reverting it would be a breaking change, we've got to FCP the initial merge.

👍 to the general change of being smarter about alignment where possible.

@rfcbot reviewed

That said, I think we need the rules written up more. Since this is a hard error, it's something that would go into a spec so that other things could make the same checks.

@rfcbot concern exact-rules-list

Looking at the PR, it looks like it's actually only [u8] and [i8], not, say, [NonZero<u8>]. And that feels weirdly inconsistent.

It reminds me of the experiments that have been done towards having niches for references, which wanted a query for alignment and size (or a sound approximation thereof) that would work even in cases where the full layout couldn't be calculated. This seems like it would like to have the same kind of thing, rather than adding a bunch of ad-hoc layout knowledge into this specific check.

traviscross · 2025-08-22T19:26:27Z

@rustbot labels +S-waiting-on-documentation

@hns1971, we'll probably want to see a PR to the Reference here documenting the new behavior. Let us know if you have any questions about that.

cc @rust-lang/lang-docs

hns1971 · 2025-08-28T04:05:24Z

Thanks for the detailed feedback!

I’ve updated the PR with the following:

Generalized the check to any [T; N]: when layout_of([T; N]) is unavailable,
we compute layout_of(T) and compare abi_align(T) to the packed alignment.
This relies on align([T]) == align(T) (length is irrelevant).
I also updated the comment with the proof sketch:
(1) &[T] -> &T implies align([T]) >= align(T)
(2) std::array::from_ref(&T) -> &[T; 1] implies align(T) >= align([T])
Hence align([T]) == align(T).
Renamed the helper to is_potentially_misaligned and switched terminology to “misaligned”.
The doc comment now explicitly states that this may return false positives, but must not
return false negatives (soundness-critical).
Merged tests into a single file and renamed it to packed-array-generic-length.rs.
The test covers:
- allowed: [u8; N], [i8; N], [NonZeroU8; N], [MaybeUninit<u8>; N],
  and #[repr(transparent)](u8) wrappers
- still error: [u16; N], [NonZeroU16; N], and #[repr(transparent)](u16) wrappers

I’ve also drafted the exact rules in the PR description and opened a Reference PR
documenting this behavior (see rust-lang/reference#1984).
Please let me know if you want the rules tweaked further or if there’s a preferred place
in the Reference to put this.

tmandry · 2025-09-10T15:28:02Z

Does this PR create a new semver hazard? For example..

// In a remote crate
struct Remote(u8);

// In local crate
#[repr(packed)]
struct Local {
  remote: Remote,
}

let remote = &Local::default().remote;

Then Remote can be changed to have a u16, which is now a semver-breaking change.

@rfcbot concern semver hazard

Otherwise, +1 for the overall idea.

@rfcbot reviewed

RalfJung · 2025-09-10T15:33:14Z

That semver hazard already exists outside of arrays. This PR just makes the logic that figures out the alignment of a type smarter: we can know the alignment of [T; N] without knowing N.

RalfJung · 2025-09-10T15:36:52Z

compiler/rustc_const_eval/src/util/alignment.rs

+            // Soundness: For any `T`, the ABI alignment requirement of `[T]` equals that of `T`.
+            // Proof sketch:
+            //  (1) From `&[T]` we can obtain `&T`, hence align([T]) >= align(T).
+            //  (2) Using `std::array::from_ref(&T)` we can obtain `&[T; 1]` (and thus `&[T]`),
+            //      hence align(T) >= align([T]).
+            // Therefore align([T]) == align(T). Length does not affect alignment.


Suggested change

// Soundness: For any `T`, the ABI alignment requirement of `[T]` equals that of `T`.

// Proof sketch:

// (1) From `&[T]` we can obtain `&T`, hence align([T]) >= align(T).

// (2) Using `std::array::from_ref(&T)` we can obtain `&[T; 1]` (and thus `&[T]`),

// hence align(T) >= align([T]).

// Therefore align([T]) == align(T). Length does not affect alignment.

// We can't compute the layout of `T`. But maybe we can still compute the alignment:

// For any `T`, the ABI alignment requirement of `[T]` and `[T; N]` equals that of `T`.

// Length does not affect alignment.

I don't think referencing standard library operations here makes much sense. Maybe we can reference the Reference wherever it defines the alignment of arrays, but TBH that doesn't seem necessary. What's relevant is to explicitly invoke the fact o that arrays are aligned like their element type, that's the one key reasoning step needed here. Let's not drown that in noise.

RalfJung · 2025-09-10T15:37:16Z

compiler/rustc_const_eval/src/util/alignment.rs

+    match ty.kind() {
+        ty::Array(elem_ty, _) | ty::Slice(elem_ty) => {
+            // Try to obtain the element's layout; if we can, use its ABI align.
+            match tcx.layout_of(typing_env.as_query_input(*elem_ty)) {


I'd recommend inlining this function at its only call site, this seems unnecessarily verbose.

If you want to make a helper function, it'd be get_type_align containing the entire match tcx.layout_of(typing_env.as_query_input(ty)) {.

RalfJung · 2025-09-10T15:38:50Z

tests/ui/packed/packed-array-generic-length.rs

+}
+
+fn main() {
+    // Run pass cases (fail cases only check diagnostics)


These never get run anyway since the file fails to compile, so this is quite pointless.

RalfJung · 2025-09-10T15:40:30Z

tests/ui/packed/packed-array-generic-length.rs

+mod pass_i8 {
+    #[repr(C, packed)]
+    pub struct S<const N: usize> {
+        buf: [i8; N],
+    }
+    pub fn run() {
+        let s = S::<4> { buf: [1, 2, 3, 4] };
+        let _ok = &s.buf[..]; // no E0793
+    }
+}
+
+mod pass_nonzero_u8 {
+    use super::*;
+    #[repr(C, packed)]
+    pub struct S<const N: usize> {
+        buf: [NonZeroU8; N],
+    }
+    pub fn run() {
+        let s = S::<3> {
+            buf: [
+                NonZeroU8::new(1).unwrap(),
+                NonZeroU8::new(2).unwrap(),
+                NonZeroU8::new(3).unwrap(),
+            ],
+        };
+        let _ok = &s.buf[..]; // no E0793
+    }
+}
+
+mod pass_maybeuninit_u8 {
+    use super::*;
+    #[repr(C, packed)]
+    pub struct S<const N: usize> {
+        buf: [MaybeUninit<u8>; N],
+    }
+    pub fn run() {
+        let s = S::<2> { buf: [MaybeUninit::new(1), MaybeUninit::new(2)] };
+        let _ok = &s.buf[..]; // no E0793
+    }
+}
+
+mod pass_transparent_u8 {
+    #[repr(transparent)]
+    pub struct WrapU8(u8);
+
+    #[repr(C, packed)]
+    pub struct S<const N: usize> {
+        buf: [WrapU8; N],
+    }
+    pub fn run() {
+        let s = S::<2> { buf: [WrapU8(1), WrapU8(2)] };
+        let _ok = &s.buf[..]; // no E0793
+    }
+}


All of these already pass before your PR, so they don't actually test what you want them to test.

You need to make the N generic, like in pass_u8. (In the fail tests it should also be generic to ensure it hits the same codepath.)

RalfJung · 2025-09-10T15:40:48Z

tests/ui/packed/packed-array-generic-length.rs

+    pub fn run() {
+        let p = PascalString::<10> { len: 3, buf: *b"abc\0\0\0\0\0\0\0" };
+        let s = bar(&p);
+        assert_eq!(s, "abc");
+    }


There's no reason I can see for this function to exist.

hns1971 added 7 commits August 15, 2025 09:53

Fix code formatting issues

19f51f2

Fix remaining code formatting issues

a2a94b9

Fix TypingEnv formatting to match rustfmt requirements

2e84ab8

Fix TypingEnv formatting to match rustfmt requirements

d8785b9

Fix TypingEnv formatting to match rustfmt requirements

dd1537c

Fix TypingEnv formatting to match rustfmt requirements

99dedb0

rustbot assigned SparrowLii Aug 15, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 15, 2025

hns1971 mentioned this pull request Aug 15, 2025

Compiler doesn’t allow creating u8 slices when using const generics #145376

Open

This comment has been minimized.

Sign in to view

fix: update is_disaligned check and tests

4620527

gralpli reviewed Aug 15, 2025

View reviewed changes

compiler/rustc_const_eval/src/util/alignment.rs Outdated Show resolved Hide resolved

tests/ui/packed/packed-array-const-generic.rs Outdated Show resolved Hide resolved

lcnr reviewed Aug 15, 2025

View reviewed changes

hns1971 added 3 commits August 18, 2025 13:16

Rename is_disaligned to is_potentially_disaligned for better semantic…

2fe4b7f

… clarity; Unified to ty::Array(element_ty, _) | ty::Slice(element_ty) => …; Modify Chinese comments to English comments

Rename is_disaligned to is_potentially_disaligned for better semantic…

f0c8e9e

… clarity

Rename is_disaligned to is_potentially_disaligned for better semantic…

4a65fdc

… clarity

hns1971 requested review from RalfJung, lcnr and gralpli August 18, 2025 07:13

hns1971 added 2 commits August 18, 2025 16:38

Add test cases to verify u8/i8 array fix

585af05

updated the comment to formally justify that align([T]) == align(T)

b37f571

lcnr reviewed Aug 22, 2025

View reviewed changes

lcnr changed the title ~~alignborrowck: allow borrowing packed arrays with ABI align <= packed align in const generics~~ do not lint when borrowing array elements from packed structs with ABI align <= packed align Aug 22, 2025

traviscross added the I-lang-radar Items that are on lang's radar and will need eventual work or consideration. label Aug 22, 2025

traviscross changed the title ~~do not lint when borrowing array elements from packed structs with ABI align <= packed align~~ Allow borrowing array elements from packed structs with ABI align <= packed align Aug 22, 2025

rustbot added the I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy; this does not imply nomination label Aug 22, 2025

rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Aug 22, 2025

traviscross removed the I-lang-easy-decision Issue: The decision needed by the team is conjectured to be easy; this does not imply nomination label Aug 22, 2025

rustbot added the S-waiting-on-documentation Status: Waiting on approved PRs to documentation before merging label Aug 22, 2025

refactor: improve get_element_alignment function for packed arrays

ef2538b

This comment has been minimized.

Sign in to view

hns1971 mentioned this pull request Aug 27, 2025

docs: add comprehensive documentation for borrowing from repr(packed)… rust-lang/reference#1984

Open

modify commit for check

8c07e0a

RalfJung reviewed Sep 10, 2025

View reviewed changes

		// For const generic arrays like [u8; CAP], we can make a reasonable assumption
		// about their alignment based on the element type.

Allow borrowing array elements from packed structs with ABI align <= packed align #145419

Are you sure you want to change the base?

Allow borrowing array elements from packed structs with ABI align <= packed align #145419

Uh oh!

Conversation

hns1971 commented Aug 15, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

rustbot commented Aug 15, 2025

Uh oh!

rustbot commented Aug 15, 2025

Uh oh!

This comment has been minimized.

gralpli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr Aug 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rustbot commented Aug 18, 2025

Uh oh!

hns1971 commented Aug 18, 2025

Uh oh!

hns1971 commented Aug 20, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lcnr commented Aug 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

traviscross commented Aug 22, 2025

Uh oh!

rfcbot commented Aug 22, 2025 • edited by rust-rfcbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lcnr commented Aug 22, 2025

Uh oh!

scottmcm commented Aug 22, 2025

Uh oh!

traviscross commented Aug 22, 2025

Uh oh!

This comment has been minimized.

hns1971 commented Aug 28, 2025

Uh oh!

tmandry commented Sep 10, 2025

Uh oh!

RalfJung commented Sep 10, 2025

Uh oh!

RalfJung Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hns1971 commented Aug 15, 2025 •

edited by rustbot

Loading

lcnr Aug 15, 2025 •

edited

Loading

lcnr Aug 18, 2025 •

edited

Loading

lcnr commented Aug 22, 2025 •

edited

Loading

rfcbot commented Aug 22, 2025 •

edited by rust-rfcbot

Loading

RalfJung Sep 10, 2025 •

edited

Loading